DOC: Using deprecated sphinx directive instead of non-standard messages in docstrings (#18928) #18934

datapythonista · 2017-12-25T00:17:58Z

closes Documentation and consistency for deprecation #18928
tests passed
passes git diff upstream/master -u -- "*.py" | flake8 --diff
whatsnew entry

pep8speaks · 2017-12-25T00:18:10Z

Hello @datapythonista! Thanks for updating the PR.

Cheers ! There are no PEP8 issues in this Pull Request. 🍻

Comment last updated on January 06, 2018 at 13:55 Hours UTC

jreback

can you add a rule in ci/lint.sh to search for DEPRECATED (so it will fail linting). looks pretty good otherwise.

jreback · 2017-12-26T21:51:20Z

scripts/find_deprecated.py

@@ -0,0 +1,34 @@
+import re


can you add text describing how to use this & purpose

jreback · 2017-12-26T21:52:19Z

scripts/api_rst_coverage.py

@@ -13,6 +13,30 @@ def class_name_sort_key(x):
        else:


can you add a note about using this?

ideally we would also add these scripts in a not in the contributing docs.

can you update for this?

jreback · 2017-12-26T21:53:32Z

pandas/core/frame.py

@@ -3728,8 +3725,8 @@ def sort_index(self, axis=0, level=None, ascending=True, inplace=False,

    def sortlevel(self, level=0, axis=0, ascending=True, inplace=False,
                  sort_remaining=True):
-        """
-        DEPRECATED: use :meth:`DataFrame.sort_index`
+        """.. deprecated:: 0.20.0


can you make the first line the simple description

jreback · 2017-12-26T21:53:40Z

pandas/core/generic.py

@@ -4090,8 +4090,8 @@ def _consolidate(self, inplace=False):
            return self._constructor(cons_data).__finalize__(self)

    def consolidate(self, inplace=False):
-        """
-        DEPRECATED: consolidate will be an internal implementation only.
+        """.. deprecated:: 0.21.0


jreback · 2017-12-26T21:53:52Z

pandas/core/generic.py

-        """
-        DEPRECATED: as_matrix will be removed in a future version.
-        Use :meth:`DataFrame.values` instead.
+        """Convert the frame to its Numpy-array representation.


jreback · 2017-12-26T21:54:11Z

pandas/core/generic.py

-        """
-        Deprecated.
-        Attempt to infer better dtype for object columns
+        """.. deprecated:: 0.18.0


same, can also remove the TODO comment (we keep a separate list of these things)

jreback · 2017-12-26T21:54:40Z

pandas/core/series.py

-        """DEPRECATED: Use ``astype(object)`` instead.
+        """
+        .. deprecated :: 0.21.1
+            Use ``astype(object) instead.


I am not 100% sure, but might need a blank line after these directives (you can build and see if they look ok)

jreback · 2017-12-26T21:55:07Z

pandas/util/_decorators.py

-    if getattr(wrapper, '__doc__', None) is not None:
-        wrapper.__doc__ = ('\n'.join(wrap(msg, 70)) + '\n'
-                           + dedent(wrapper.__doc__))
+    msg = msg or 'Use `{alt_name}` instead.'


can you add a comment on what you are doing here

jreback · 2017-12-26T21:57:09Z

also would love some tests in contributing.rst on how to depreciate things (e.g. how to use the decorator is prob enough)

datapythonista · 2017-12-27T01:20:49Z

Thanks for the review @jreback. I updated the pull request, addressing all your comments. Please let me know if there is anything that is not yet clear, or that can be improved.

Regarding the blank line after the directive, it doesn't make a difference for sphinx. In both cases the comment about the deprecation is shown inline.

jreback

small comments. ping on green.

jreback · 2017-12-27T19:32:59Z

pandas/core/series.py

@@ -911,8 +915,7 @@ def repeat(self, repeats, *args, **kwargs):
                                 index=new_index).__finalize__(self)

    def reshape(self, *args, **kwargs):
-        """
-        .. deprecated:: 0.19.0
+        """.. deprecated:: 0.19.0


can you fix this one?

jreback · 2017-12-27T19:33:09Z

pandas/core/series.py

@@ -971,15 +972,13 @@ def _get_value(self, label, takeable=False):
    _get_value.__doc__ = get_value.__doc__

    def set_value(self, label, value, takeable=False):
-        """
+        """.. deprecated:: 0.21.0


here as well

jreback · 2017-12-27T19:34:23Z

ci/lint.sh

@@ -117,6 +117,10 @@ if [ "$LINT" ]; then
        fi
    done
    echo "Check for incorrect sphinx directives DONE"
+
+    echo "Check for deprecated messages without sphinx directive"
+    grep -R --include="*.py" --include="*.pyx" DEPRECATED pandas


can you also look for Deprecated (capitalize first letter), and maybe DEPRECATE

datapythonista · 2017-12-27T19:51:14Z

Sure, thanks for the comments @jreback .

I'm also implementing a more exhaustive version of find_deprecated.py. I think it makes sense to know your opinion about it before continuing.

What I think it would be useful is to have a script that detects all deprecated stuff, so it can be completely removed at the right time. That's what find_deprecated does, but just for methods of Series, DataFrame and Panel, and assuming they got the .. deprecated:: directive in the docstring.

What I'm implementing now is a version that checks all python files in the project, looks for FutureWarning, and checks the docstrings of everything in the file (the module itself, classes, functions,...), and finds what has been deprecated.

This way, if when releasing pandas 0.22, everything that was deprecated before 0.19 needs to go away, it can be detected in an easy and reliable way.

Does this make sense?

jreback · 2017-12-27T19:57:48Z

this would be fine as a double check: #18934 (comment)

We keep a nice annotated list: #6581
and removals: #13777

datapythonista · 2017-12-27T20:13:47Z

Ah, thanks for pointing it out. I checked for the list in the code, didn't know it was in github.

Then, I think I'll update the pull request with your comments, removing the find_deprecated.py script. Later on I'll implement the script, probably in a repo separate from pandas (there is nothing pandas specific in it), and run it to see if there is nothing missing in these tickets.

jreback · 2017-12-27T20:19:37Z

@datapythonista that would be great!

datapythonista · 2017-12-27T22:12:25Z

@jreback fixed items in last review. In lint.sh I added DEPRECATE, DEPRECATED and Deprecated, but only if they finish with colon, comma or dot, as it was capturing DeprecatedModule for example.

jreback

couple of comments

jreback · 2017-12-30T23:58:46Z

scripts/api_rst_coverage.py

@@ -13,6 +13,30 @@ def class_name_sort_key(x):
        else:


can you update for this?

jreback · 2017-12-30T23:59:12Z

pandas/core/sparse/series.py

@@ -256,8 +256,8 @@ def npoints(self):
    def from_array(cls, arr, index=None, name=None, copy=False,
                   fill_value=None, fastpath=False):
        """
-        DEPRECATED: use the pd.SparseSeries(..) constructor instead.
-


can you add the 1-liner description here (IOW the deprecated should not be the first line)

jreback · 2017-12-30T23:59:21Z

pandas/core/series.py

@@ -2978,8 +2979,8 @@ def dropna(self, axis=0, inplace=False, **kwargs):
                return self.copy()

    def valid(self, inplace=False, **kwargs):
-        """DEPRECATED. Series.valid will be removed in a future version.


add the 1 liner here

jreback · 2017-12-30T23:59:28Z

pandas/core/series.py

@@ -2120,8 +2120,8 @@ def nsmallest(self, n=5, keep='first'):
        return algorithms.SelectNSeries(self, n=n, keep=keep).nsmallest()

    def sortlevel(self, level=0, ascending=True, sort_remaining=True):
-        """
-        DEPRECATED: use :meth:`Series.sort_index`
+        """.. deprecated:: 0.20.0


datapythonista · 2018-01-03T00:40:35Z

@jreback, I made the requested changes. I agree that the scripts should be added to a note in the contributing documentation, but I'd do that in a separate request if you agree. That's unrelated to this PR, and some standardization of the scripts will be probably required. So I don't think it's a small change.

jreback · 2018-01-04T00:37:00Z

just needs a rebase and looks ok.

datapythonista · 2018-01-04T00:46:49Z

Rebased. Thanks @jreback

jreback · 2018-01-04T00:48:32Z

lgtm. ping on green.

datapythonista · 2018-01-04T01:02:59Z

@jreback, am I right in assuming that all the stuff deprecated in 0.19 and before can be deleted? If that's the case, I'll submit a PR leaving only the methods and functions with deprecation warning added in 0.20, 0.21 and 0.22.

jreback · 2018-01-04T01:18:15Z

things before 0.19.0 should already be removed (except for - couple of ones)

topper-123 · 2018-01-06T00:11:20Z

I submittet the PR's to deprecate as_matrix, asobject and valid. These will be deprecated in 0.23. Currently you've marked them as deprecated in 0.21.1/0.21.2.

datapythonista · 2018-01-06T11:54:55Z

Thanks for pointing out @topper-123. It looks like I misunderstood something about the git workflow. For as_matrix for example, I saw it was merged to master in November 26th, and that 0.21.1 was the next next version released (on December 12), so I assumed everything in master was released, which doesn't seem right.

I'll compare all the versions I added with #6581 and with github tags, and fix the wrong ones.

Thanks for reporting this!

…es in docstrings (#18928)

datapythonista · 2018-01-06T14:07:33Z

@topper-123 I reviewed all the deprecation versions I added, with the release where the commit was introduced (checking git tag --contains <commit>). All them should be correct now. I added the version 0.23.0 to the ones that haven't been released. They match the ones you mentioned.

@jreback, this should be ready to be merged now.

Just to let you know, compared to the changes you already reviewed, I removed from the docstring the Timestamp.offset attribute in pandas/_libs/tslibs/timestamps.pyx. The attributed was removed from the code in fb95f7f but not from the docstring. For the rest is the same, with updated deprecation versions.

Thanks to both!

jreback · 2018-01-06T17:15:32Z

thanks @datapythonista very nice!

jorisvandenbossche

@datapythonista some review after the fact. In general I am certainly +1 on making how we use things more consistent. But I personally like the "DEPRECATED" in the first one-liner for practical reasons.

jorisvandenbossche · 2018-01-11T16:25:07Z

pandas/core/frame.py

+        """Read CSV file.
+
+        .. deprecated:: 0.21.0
+            Use :func:`pandas.read_csv` instead.


For this case I actually don't find this an improvement. For deprecated methods, I find it useful that this is clear from the one-liner summary this it is deprecated, so you directly see this in the online api summary tables (on api.rst or on the DataFrame docstring page).

We could also do both?

(I know we don't do this consistently for all deprecated methods as well, but from_csv seems like a more important one)

jorisvandenbossche · 2018-01-11T16:25:27Z

pandas/core/frame.py

@@ -3737,12 +3734,13 @@ def sort_index(self, axis=0, level=None, ascending=True, inplace=False,

    def sortlevel(self, level=0, axis=0, ascending=True, inplace=False,
                  sort_remaining=True):
-        """
-        DEPRECATED: use :meth:`DataFrame.sort_index`


jorisvandenbossche · 2018-01-11T16:26:11Z

pandas/core/generic.py

@@ -4108,8 +4108,11 @@ def _consolidate(self, inplace=False):
            return self._constructor(cons_data).__finalize__(self)

    def consolidate(self, inplace=False):
-        """
-        DEPRECATED: consolidate will be an internal implementation only.
+        """Compute NDFrame with "consolidated" internals (data of each dtype


For future reference, please never use 'NDFrame' is user-facing docstrings. Users should not be aware what that means.

jorisvandenbossche · 2018-01-11T16:30:14Z

pandas/core/generic.py

@@ -4160,11 +4163,10 @@ def _get_bool_data(self):
    # Internal Interface Methods

    def as_matrix(self, columns=None):
-        """
-        DEPRECATED: as_matrix will be removed in a future version.


datapythonista · 2018-01-11T17:06:12Z

Thanks a lot for you comments @jorisvandenbossche, I see your point, didn't think about it.

My main motivation for standardizing the docstrings is to be able to automatically detect the deprecated methods. The api_rst_coverage.py and list_future_warnings.sh scripts benefit from it already.

I see two solutions here:

Given the fact that now it can easily be detected that a module is deprecated by code, check it while generating api_rst.html and the DataFrame page (I guess this needs to be done in sphinx)
Add the DEPRECATED word as the first word in all deprecated methods, besides the .. deprecated:: directive (which also provides the version)

I prefer 1, as seems the "right" way to do it. Surely more complex to implement now, but looks cleaner once implemented.

What do you think?

For the NDFrame in the docstring summary, I think I copied that summary from the called function, and didn't think NDFrame wasn't part of the public API. I'll fix it and send a PR.

Thanks!

jorisvandenbossche · 2018-01-11T17:15:25Z

Doing that in sphinx will probably some overkill IMO, so a pragmatic solution is to do both. I don't think that is a problem, as the .. deprectated:: directive gives then additional information on when it is deprecated.

shoyer · 2018-03-14T17:54:04Z

Out of curiosity, how does the .. deprecated:: macro appear in generated docs? I hope it's highly visible in some way, e.g., a big red box?

TomAugspurger · 2018-03-14T18:16:02Z

#20349

sinhrks added Deprecate Functionality to remove in pandas Docs labels Dec 25, 2017

jreback requested changes Dec 26, 2017

View reviewed changes

jreback requested changes Dec 27, 2017

View reviewed changes

jreback mentioned this pull request Dec 27, 2017

DOC: Update doc strings to show pd.Panel has been deprecated #18956

Merged

jreback requested changes Dec 30, 2017

View reviewed changes

datapythonista changed the title ~~Deprecation~~ DOC: Using deprecated sphinx directive instead of non-standard messages in docstrings (#18928) Jan 3, 2018

jreback added this to the 0.23.0 milestone Jan 4, 2018

jreback approved these changes Jan 4, 2018

View reviewed changes

DOC: Using deprecated sphinx directive instead of non-standard messag…

0673b51

…es in docstrings (#18928)

jreback merged commit 1d32264 into pandas-dev:master Jan 6, 2018

jorisvandenbossche reviewed Jan 11, 2018

View reviewed changes

datapythonista mentioned this pull request Jan 13, 2018

Adding (DEPRECATED) prefix to deprecated objects summary in the documentation #19220

Merged

4 tasks

DOC: Using deprecated sphinx directive instead of non-standard messages in docstrings (#18928) #18934

DOC: Using deprecated sphinx directive instead of non-standard messages in docstrings (#18928) #18934

Conversation

datapythonista commented Dec 25, 2017

pep8speaks commented Dec 25, 2017 • edited Loading

Comment last updated on January 06, 2018 at 13:55 Hours UTC

jreback left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jreback commented Dec 26, 2017

datapythonista commented Dec 27, 2017

jreback left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

datapythonista commented Dec 27, 2017

jreback commented Dec 27, 2017

datapythonista commented Dec 27, 2017

jreback commented Dec 27, 2017

datapythonista commented Dec 27, 2017

jreback left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

datapythonista commented Jan 3, 2018

jreback commented Jan 4, 2018

datapythonista commented Jan 4, 2018

jreback commented Jan 4, 2018

datapythonista commented Jan 4, 2018

jreback commented Jan 4, 2018

topper-123 commented Jan 6, 2018

datapythonista commented Jan 6, 2018

datapythonista commented Jan 6, 2018

jreback commented Jan 6, 2018

jorisvandenbossche left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

datapythonista commented Jan 11, 2018

jorisvandenbossche commented Jan 11, 2018

shoyer commented Mar 14, 2018

TomAugspurger commented Mar 14, 2018

pep8speaks commented Dec 25, 2017 •

edited

Loading